Skip to content

Conversation

vbvictor
Copy link
Contributor

@vbvictor vbvictor commented Sep 6, 2025

Closes #146482.

@llvmbot
Copy link
Member

llvmbot commented Sep 6, 2025

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Baranov Victor (vbvictor)

Changes

Closes #146482.


Full diff: https://github.com/llvm/llvm-project/pull/157306.diff

2 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+1-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2)
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index bef3b938b5afd..ed9b195f0dbde 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -102,8 +102,7 @@ Configuration files:
 )");
 
 const char DefaultChecks[] = // Enable these checks by default:
-    "clang-diagnostic-*,"    //   * compiler diagnostics
-    "clang-analyzer-*";      //   * Static Analyzer checks
+    "clang-diagnostic-*";    //   * compiler diagnostics
 
 static cl::opt<std::string> Checks("checks", desc(R"(
 Comma-separated list of globs with optional '-'
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e1b6daf75457d..d97efe667f98e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -139,6 +139,8 @@ Improvements to clang-tidy
   scripts by adding the `-hide-progress` option to suppress progress and
   informational messages.
 
+- Removed `clang-analyzer-*` check from default checks in :program:`clang-tidy`.
+
 New checks
 ^^^^^^^^^^
 

Copy link
Contributor

@carlosgalvezp carlosgalvezp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I wonder if we need a 2-release cycle for this, after all people might silently lose an important part of clang-tidy coverage and the note in the release notes could be hard to spot in the middle of the other notes.

Perhaps having a section for "Breaking changes" or otherwise "Stuff you need to take action on" would help.

Would be good to hear what other reviewers think!

@vbvictor
Copy link
Contributor Author

vbvictor commented Sep 7, 2025

Perhaps having a section for "Breaking changes" or otherwise "Stuff you need to take action on" would help.

I support this, another solution could be bold text or preamble like "Important ⚠️".
But I think dedicated section is the best.

@vbvictor
Copy link
Contributor Author

Perhaps having a section for "Breaking changes" or otherwise "Stuff you need to take action on" would help.

Filed #158434, once it is merged we could mark it as a breaking change.

Copy link

github-actions bot commented Sep 21, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@vbvictor vbvictor force-pushed the disable-default-csa-checks branch from 54d4521 to 4ab5499 Compare September 21, 2025 21:02
@carlosgalvezp
Copy link
Contributor

Now that #158434 is merged, should we move the change to the release notes to that section?

@vbvictor vbvictor force-pushed the disable-default-csa-checks branch from f11b956 to 57c0b1e Compare October 2, 2025 20:19
@vbvictor
Copy link
Contributor Author

vbvictor commented Oct 2, 2025

Now that #158434 is merged, should we move the change to the release notes to that section?

Yes, I finally rebased on main and added new section

@vbvictor vbvictor force-pushed the disable-default-csa-checks branch from 57c0b1e to 534a01f Compare October 2, 2025 20:25
Copy link
Contributor

@carlosgalvezp carlosgalvezp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@5chmidti
Copy link
Contributor

5chmidti commented Oct 3, 2025

But do we want to mention the change twice in the change log? In another PR we are only mentioning the breaking change in the breaking changes section, not again in the improvement's section.

@vbvictor
Copy link
Contributor Author

vbvictor commented Oct 3, 2025

But do we want to mention the change twice in the change log? In another PR we are only mentioning the breaking change in the breaking changes section, not again in the improvement's section.

I initially thought to duplicate entries just in case if someone would read only clang-tidy part. I think it's better to have in both

@5chmidti
Copy link
Contributor

5chmidti commented Oct 3, 2025

I totally agree with the consistency. I personally think once is fine, but twice won't hurt.

The other PR I mentioned would be #161064

@vbvictor
Copy link
Contributor Author

vbvictor commented Oct 3, 2025

I totally agree with the consistency. I personally think once is fine, but twice won't hurt.

The other PR I mentioned would be #161064

Oh, must have missed that, we probably should add in tidy release notes for consistency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang-tidy] clang-analyzer-* checks should be disabled by default
4 participants